Patch: AJ's pre-commit updates #49846
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
aaronsteers
changed the title
| <excludes> | ||
| <exclude>**/non_formatted_code/*</exclude> | ||
| </excludes> |
There was a problem hiding this comment.
This was necessary because this file would otherwise be formatted by spotless when running pre-commit with the --all-files flag.
Since spotless doesn't receive individual files from pre-commit, the pre-commit exclude list doesn't apply.
|
octavia-squidington-iii
added
area/connectors
|
| ^airbyte-ci/connectors/metadata_service/lib/tests/fixtures/.*/invalid/.*$| | ||
| ^.*?/airbyte-ci/connectors/pipelines/tests/test_format/non_formatted_code/.*$| |
There was a problem hiding this comment.
Formatters seem inconsistent on whether they will accept ^ or ^.*?/ at the start of the path - and this may also have something to do with the relative path or path that the file is cloned at. Adding both ways solves this in the case above that I found was inconsistently applied.
There was a problem hiding this comment.
Yeah but I think we can live with that. Ultimately I think we can remove the tests for those unformatted things because, well, those are FORMATTER TESTS
| @@ -25,8 +27,7 @@ repos: | |||
| hooks: | |||
| - id: black | |||
| language_version: python3.10 | |||
| verbose: true | |||
| args: ["--verbose"] | |||
| args: [--config=pyproject.toml] | |||
There was a problem hiding this comment.
The --config-pyproject.toml arg might not be needed, but consistency seemed worthwhile.
Also - I'm okay bringing back verbose if we need it, but was very spammy on my machine when --all-files was passed as an arg.
(I also tried sending an explicit config path to isort - but this had an adverse affect so I reverted it.)
| ^.*?/pnpm-lock\.yaml$| | ||
| ^.*?/source-amplitude/unit_tests/api_data/zipped\.json$| | ||
|
|
What
Patches and stacks with
How
airbyte-ciwithpre-commitin the format check and format fix CI workflows.--all-filesarg is used.airbyte-ciso format check and fix can run without escalated privileges./format-fixto run on forks (in theory, but will needs to be tested post-merge).Example Workflow Runs:
Review guide
User Impact
Can this PR be safely reverted and rolled back?